[uss_qualifier/scenarios/netrid/nominal_behavior] Add checks for UA type in SP (NET0260) #865
Conversation
| "UA type is present and consistent with injected one", | ||
| participants, | ||
| ) as check: | ||
| if observed_val is None: |
There was a problem hiding this comment.
It seems like this is only valid if injected_val is not None. And, I'm not sure it would be wrong for a USS to report NotDeclared if injected_val was None.
There was a problem hiding this comment.
The way I understand the standard is that the USS must always report the UA type because it is a required field, and if it is unknown (i.e. nothing was injected), it must report NotDeclared. Thus why I included this failure when it is not exposed.
However on the other side the observation/DP interface does not have marked as required the field aircraft_type, but the rid/SP interface does have it required.
I'm not sure how to proceed given that.
Shall we accept omission of that field?
Shall we discriminate between DP and SP for failing that check when field is not exposed?
There was a problem hiding this comment.
Ah, my mistake -- this is the SP-DP interface and this is a required field, so you're right that None is simply invalid regardless of injection. None should not be allowed past the parser, but doesn't hurt to double check here.
The DP under test can't be at fault because uss_qualifier is acting as DP when querying the SP, so the DP under test isn't even involved.
There was a problem hiding this comment.
None should not be allowed past the parser
Indeed.
The DP under test can't be at fault because uss_qualifier is acting as DP when querying the SP, so the DP under test isn't even involved.
I'm not sure your comment was regarding that, but do note that the function _evaluate_ua_type implemented here is meant to be reused for both SP (this PR) and DP (PR #866). The caller does have to adapt the participants depending on which is tested.
There was a problem hiding this comment.
observed_val being None is invalid for the SP API, but it's valid for the Observation API so I think this is good for this PR but possibly not for future ones.
| injected_val_enum = injection.UAType(injected_val) | ||
| except ValueError: | ||
| if observed_val != injection.UAType.NotDeclared: | ||
| check.record_failed( |
There was a problem hiding this comment.
I think this should actually raise an exception -- the injection attempt should be rejected if uss_qualifier doesn't follow the injection API and I think the injection API has the injected value as either omitted or a valid UAType, so there should be no way for us to reach this failure if uss_qualifier is working properly.
There was a problem hiding this comment.
OK will change (and also take into account for future checks) that we should raise exceptions in cases where we injected invalid data.
There was a problem hiding this comment.
Actually what I propose is to just adapt that check to fail when there is no injected value and that the observed value is different from NotDeclared. I'm not sure that we should start explicitly verifying the correctness of injected values (here we now don't need to try parsing injected_val anymore).
There was a problem hiding this comment.
I think we want to fail as soon as possible whenever a contract is violated and injecting an invalid UAType violates the injection API contract. If we haven't already failed due to that contract failure, I think it's appropriate to fail here. Basically, my understanding is that we're evaluating a value in the SP or observation API in C1 or C2 below (where the test designer or uss_qualifier is at fault). Probably we shouldn't reach that point, but if we do, I don't think the failure responsibility somehow transfers to the SP or DP. I think the appropriate way to indicate a uss_qualifier or test design problem is to raise an exception.
The table below is my understanding of who is at fault for what failure, and the priority of rows goes from top to bottom. Note the difference between C3 and C8 when reusing the same function to evaluate SP API responses and Observation API responses.
| C | Data to inject | Injection API | SP API | Observation API | Responsible entity | Failure |
|---|---|---|---|---|---|---|
| 1 | [invalid] | [any] | [any] | [any] | Test designer | Invalid test data provided |
| 2 | [none or valid] | [anything that doesn't match data to inject, including invalid] | [any] | [any] | uss_qualifier developer | Test data not injected accurately |
| 3 | [none or valid] | [matching data to inject] | [none] | [any] | SP | API-required field not provided |
| 4 | [none] | NotDeclared | [any] | [any] | uss_qualifier developer | Test data not injected accurately |
| 5 | [none or valid] | [matching data to inject] | [invalid] | [any] | SP | SP API contract violated |
| 6 | [none] | [none] | [anything other than NotDeclared] | [any] | SP | UA type not communicated correctly |
| 7 | [none or valid] | [matching data to inject] | [valid, but not corresponding to injected value] | [any] | SP | UA type not communicated correctly |
| 8 | [none or valid] | [matching injected data] | [valid] | [none] | N/A | No failure; reporting UA type to observers is not required by API |
| 9 | [none or valid] | [matching injected data] | [valid] | [invalid] | DP | Observer API contract violated |
| 10 | [none or valid] | [matching injected data] | [valid] | [valid, but not corresponding to SP value] | DP | SP information incorrectly reported to observer |
monitoring/uss_qualifier/scenarios/astm/netrid/common_dictionary_evaluator.py
Show resolved
Hide resolved
monitoring/uss_qualifier/scenarios/astm/netrid/common_dictionary_evaluator.py
Outdated
Show resolved
Hide resolved
mickmis
force-pushed
the
rid/uatype
branch
2 times, most recently
from
52813b2 to
dd46406
Compare
…ype in SP (NET0260)
| "UA type is present and consistent with injected one", | ||
| participants, | ||
| ) as check: | ||
| if observed_val is None: |
There was a problem hiding this comment.
observed_val being None is invalid for the SP API, but it's valid for the Observation API so I think this is good for this PR but possibly not for future ones.
| self, | ||
| injected_val: Optional[str], | ||
| observed_val: Optional[str], | ||
| participants: List[ParticipantID], |
There was a problem hiding this comment.
It would probably be helpful to clarify which participant(s) these are to make evaluation of the checks more straightforward -- perhaps "participant providing the API through which the value was observed" which would cover the SP-DP API in this PR and the Observation API in future PRs.
This PR adds checks for UA type in SP response. This adds coverage for requirements v22a.NET0260,Table1,2 and v19.NET0260,Table1,3.